Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vmstat: Add root element to libxo output #1330

Closed
wants to merge 1 commit into from
Closed

Conversation

bramton
Copy link
Contributor

@bramton bramton commented Jul 17, 2024

Partial fix for Bug 254635.

On a side note: The conventions around libxo output seem to be a bit inconsistent. For instance netstat and jls do not use the program name as root element, but use statistics and jail-information instead. The procstat program uses the getprogname() function to set the name of the root element, whilst arp hard codes the program name. What are the best practices regarding libxo output?

bramton added a commit to bramton/freebsd that referenced this pull request Jul 19, 2024
Current libxo output does not have a root element. Valid XML requires a single
root element. This commit adds this root element.

PR: 254635
Pull Request: freebsd#1330
@bramton bramton changed the title Added root element to vmstat libxo output vmstat: Add root element to vmstat libxo output Jul 19, 2024
@bramton bramton changed the title vmstat: Add root element to vmstat libxo output vmstat: Add root element to libxo output Jul 19, 2024
bramton added a commit to bramton/freebsd that referenced this pull request Jul 19, 2024
Current libxo output does not have a root element. Valid XML requires a single
root element. This commit adds this root element.

PR: 254635
Pull Request: freebsd#1330
@jlduran
Copy link
Member

jlduran commented Jul 27, 2024

I wonder if such change would warrant a version bump?

@bramton
Copy link
Contributor Author

bramton commented Aug 9, 2024

Agreed. Version number has been bumped.

@bsdimp
Copy link
Member

bsdimp commented Aug 23, 2024

I think this is fine...

In an ideal world, we'd document the schemas and versions in the man page, but that's not done elsewhere.

@markjdb
Copy link
Member

markjdb commented Aug 23, 2024

This looks good to me, thank you. The two commits can be squashed together IMO.

What are the best practices regarding libxo output?

As far as I know, we don't have any documented conventions beyond what's in the libxo documentation itself. I do think it makes sense to have a root element, and that's what most programs do. It would be nice to document these version bumps in the man page as well, explaining what changed, etc., but I think this patch is ok as it is.

Current libxo output does not have a root element. Valid XML requires a single
root element. This commit adds this root element.

libxo output version bumped accordingly.

PR: 254635
Pull Request: freebsd#1330
@bramton
Copy link
Contributor Author

bramton commented Aug 27, 2024

This looks good to me, thank you. The two commits can be squashed together IMO.

Done!

@delphij delphij self-assigned this Sep 3, 2024
@delphij
Copy link
Member

delphij commented Sep 3, 2024

Merged as c7dd97e , thanks!

@delphij delphij closed this Sep 3, 2024
freebsd-git pushed a commit that referenced this pull request Sep 3, 2024
Current libxo output does not have a root element. Valid XML requires a single
root element. This commit adds this root element.

The libxo output version bumped accordingly.

PR:		254635
MFC after:	1 week
Pull Request: #1330
@bsdimp bsdimp added the merged label Sep 3, 2024
freebsd-git pushed a commit that referenced this pull request Sep 10, 2024
Current libxo output does not have a root element. Valid XML requires a single
root element. This commit adds this root element.

The libxo output version bumped accordingly.

PR:		254635
Pull Request: #1330

(cherry picked from commit c7dd97e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants